feat: codex exec auto-subscribe to new threads#9821
Conversation
jif-oai
changed the title
jif-oai
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5164282cc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex-rs/exec/src/lib.rs
Outdated
| CodexStatus::Running => continue, | ||
| CodexStatus::InitiateShutdown => { | ||
| thread.submit(Op::Shutdown).await?; | ||
| } | ||
| CodexStatus::Shutdown => { | ||
| break; | ||
| } | ||
| CodexStatus::Shutdown => break, |
There was a problem hiding this comment.
With auto-subscribed threads, a background thread that finishes will emit ShutdownComplete (each session ends with this event), process_event maps it to CodexStatus::Shutdown, and this loop breaks unconditionally even if the primary thread is still running. That can terminate codex-exec early and drop the remaining output when a collaboration thread completes before the main session. Consider only breaking on Shutdown when thread_id == primary_thread_id (similar to the existing guard for InitiateShutdown).
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 639ce3d606
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if thread_id != primary_thread_id && matches!(&event.msg, EventMsg::TurnComplete(_)) { | ||
| continue; | ||
| } | ||
| let shutdown = event_processor.process_event(event); | ||
| if thread_id != primary_thread_id && matches!(shutdown, CodexStatus::InitiateShutdown) { |
There was a problem hiding this comment.
With auto-subscribe, events from non-primary threads now flow through the single event_processor (process_event(event) here). The JSON/human processors maintain global per-turn state (e.g., running_commands, running_mcp_tool_calls, running_todo_list) and handle_task_complete() flushes all of it on any TurnComplete. If a secondary thread starts a command and the primary turn completes first, that command will be emitted as completed and removed; when its real ExecCommandEnd arrives it is dropped as “without matching begin”, losing output/exit code and corrupting logs. Consider a processor per thread or keying running_* state by thread_id and only flushing for the matching thread.
Useful? React with 👍 / 👎.
2 participants
No description provided.